miner: simplify code for simulating bids#3135
Merged
zzzckck merged 1 commit intobnb-chain:developfrom Jun 3, 2025
buddh0:simplify-code-for-simulating-bids
Merged
miner: simplify code for simulating bids#3135zzzckck merged 1 commit intobnb-chain:developfrom buddh0:simplify-code-for-simulating-bids
zzzckck merged 1 commit intobnb-chain:developfrom
buddh0:simplify-code-for-simulating-bids
Conversation
buddh0
commented
May 30, 2025
| if bestBid == nil { | ||
| winResult := "true[first]" | ||
| if bestBidOnStart != nil { | ||
| // new block was imported, so the bestBidOnStart was cleared, the bid will be stale and useless. |
Contributor
Author
There was a problem hiding this comment.
won't happen, clear bestBid delayed a lot of blocks(TriesInMemory).
buddh0
commented
May 30, 2025
| } | ||
|
|
||
| // only recommit last best bid when newBidCh is empty | ||
| if len(b.newBidCh) > 0 { |
Contributor
Author
There was a problem hiding this comment.
recommit logic not depend on bid simulation.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the bid simulation logic by removing redundant code and streamlining the best bid update and logging processes. The changes focus on removing the previously used bestBidOnStart variable, revising the logging conditions, and adjusting the bid comparison logic.
- Removed bestBidOnStart and related stale bid handling logic.
- Modified logging conditions to always log with simElapsed and adjusted bid comparison from >= to >.
- Updated the best bid selection block to combine nil-check and reward comparison.
Comments suppressed due to low confidence (3)
miner/bid_simulator.go:864
- [nitpick] Since the best bid is updated here when bestBid is nil or a better bid is received, consider adding a comment to explain how this update integrates with the earlier logging logic for clarity.
b.SetBestBid(bidRuntime.bid.ParentHash, bidRuntime)
miner/bid_simulator.go:845
- Review the change in logging logic: previously winResult was dynamically adjusted based on bestBidOnStart, but now it is always reported as 'true[first]' when bestBid is nil. Confirm that this simplified approach meets the intended simulation outcome.
log.Info("[BID RESULT]", "win", winResult, "builder", bidRuntime.bid.Builder, "hash", bidRuntime.bid.Hash().TerminalString(), "simElapsed", time.Since(startTS))
miner/bid_simulator.go:848
- Ensure that updating the condition from '>=' to '>' for comparing packedBlockReward is intended, since equal bids will no longer be considered improvements.
"win", bidRuntime.packedBlockReward.Cmp(bestBid.packedBlockReward) > 0,
galaio
approved these changes
Jun 3, 2025
WTDuck2255
approved these changes
Jun 3, 2025
zzzckck
approved these changes
Jun 3, 2025
sysvm
pushed a commit
to sysvm/bsc
that referenced
this pull request
Jun 16, 2025
This was referenced Jun 16, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
miner: simplify code for simulating bids
Rationale
mini logic changes, clear more than 20+ lines code
Example
add an example CLI or API response...
Changes
Notable changes: